-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Included sample python code for DIDs and Price Oracles #2932
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for providing these! It's nice to have simple, working code for basic functions of each feature like this. I have just a few requests to polish them up before we publish them.
Could you include the requirements.txt
file for these samples? I'm guessing it's just a one-liner with xrpl>=3.0.0
or something like that, but it helps to make that explicit especially since some relatively recent versions of xrpl-py don't support these features.
Also, the DID samples could use a README. For reference, the "Code Samples" page on XRPL.org uses the title and the first paragraph in its card representation, so the first paragraph should be a short (1-2 sentence) description of the code sample with no formatting. If you want to provide additional context like how to run the samples, it can go in later paragraphs or in a separate README at the language level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to have even a one-sentence summary beyond the title in this file.
Maybe a second paragraph with basic usage instructions too.
_code-samples/did/py/did_delete.py
Outdated
|
||
|
||
# restore an account that has an existing DID | ||
account_did_creator = Wallet.from_seed(seed="sEdTP3iaY4PE9vYm2LBCsrY4LaTpU7m") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not advisable to include hard-coded seeds in code that's committed to the repo, even if they're not holding any real value. Someone might use the seed for real purposes and get themself in trouble, or someone may change the settings on the account in unpredictable ways, making the samples behave unexpectedly.
In this case I was able to run the sample code successfully once, and now it'll return tecNO_ENTRY
for everyone from here on.
Since this set of code also includes a DIDSet example that funds an account with the faucet, I would have that sample output the seed and address it generated, and then have this section use input()
to ask the user to paste the seed.
Then you would have to explain in the README (that doesn't currently exist) that they have to create a DID with did_set.py
before they have something to delete.
No description provided.